[C팀] 백엔드파트 코드리뷰용 PR #76

anxi01 · 2024-11-03T15:03:33Z

개발파트 심사위원 코드리뷰용 PR입니다.

바쁜 와중에 코드 리뷰해 주셔서 정말 감사드립니다.

devxb

클린아키텍처, DDD와 같은 개념이 쉽지 않았을거 같은데, 여러가지 시도를 해주신것 같아서 재미있게 봤습니다. 수고하셨습니다 👍

질문 주신내용중 하나인, 3. 프로젝트의 품질을 향상하고 싶습니다. 에 대해서는 여기에 답해보도록 할게요.

사실 어떤 방향으로의 품질 향상인지가 약간 추상적이라 생각나는대로 적어보겠습니다 🙇‍♂️

이미 알고계실수도 있을거 같은데, AggregateRoot, Domain service, Application Service 에 대해서 공부해보셔도 좋을거 같습니다.
클린아키텍처가 과연 좋을까요?
테스트작성 방법중에서 Describe-Context-It 이라는 패턴이 있습니다. DDD를 하게 되면 도메인이 비대해지면서 테스트코드도 하나의 클래스에 몰려있는 경우가 발생하는데요. 이때, Describe-Context-It을 쓰면 늘어나는 테스트코드를 관리하기 쉬워집니다.
참고: https://johngrib.github.io/wiki/junit5-nested/
sonar cloud를 쓰면 lint를 어느정도 자동화 해서 관리해줍니다. https://www.sonarsource.com/products/sonarcloud/
application service를 활용하다보면 모든 로직을 트랜잭션으로 묶지 못하는 경우가 발생하는데요. 이때, 업데이트를 일부만 성공하게 된다면 데이터 정합성은 깨지게 됩니다. 이런 상황을 어떻게 방지할 수 있을까요?
각 도메인 끼리는 어떻게 호출을 해야할까요?
API가 중복(따닥) 호출되면 어떻게 될까요? 어떻게 멱등한 API를 구현할 수 있을까요?
일단 유저를 모으는게 중요한것 같아요. (안된다면 기술을 해야겠지만요) 같은 개선을 하더라도 유저관점에서 유의미한 개선을 했을때 오는 임팩트가 크다고 생각합니다.

더 많을거 같은데 당장 생각나는건 이정도네요 ㅎㅎ

devxb · 2024-11-06T13:16:44Z

Dockerfile-local

+
+EXPOSE 8080
+ENTRYPOINT ["java"]
+CMD ["-jar", "backend.jar"]


사소한것이지만 파일의 마지막에 개행을 넣으시면 아래 사진과 같은것을 안 볼 수 있습니다!

감사합니다!!

devxb · 2024-11-06T13:20:05Z

build.gradle

 dependencies {
-	implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
-	implementation 'org.springframework.boot:spring-boot-starter-validation'
+	// WEB
 	implementation 'org.springframework.boot:spring-boot-starter-web'
-	compileOnly 'org.projectlombok:lombok'
-	developmentOnly 'org.springframework.boot:spring-boot-devtools'
-	annotationProcessor 'org.projectlombok:lombok'
+
+	// Testing
 	testImplementation 'org.springframework.boot:spring-boot-starter-test'
+	testImplementation 'org.springframework.security:spring-security-test'
 	testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+	testImplementation 'com.h2database:h2'
+
+	// Development tools (for local development only)
+	developmentOnly 'org.springframework.boot:spring-boot-devtools'
+
+	// DB, JPA
+	runtimeOnly 'com.mysql:mysql-connector-j'
+	implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
+
+	// Lombok (code simplification)
+	compileOnly 'org.projectlombok:lombok'
+	annotationProcessor 'org.projectlombok:lombok'
+
+	// Validation (for request/response validation)
+	implementation 'org.springframework.boot:spring-boot-starter-validation'
+
+	// WebSocket (for real-time communication)
+	implementation 'org.springframework.boot:spring-boot-starter-websocket'
+
+	// Swagger-UI
+	implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.0.2'
+
+	// JWT
+	implementation 'io.jsonwebtoken:jjwt-api:0.12.6'
+	runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.12.6'
+	runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.12.6'


마찬가지로 사소한것이지만, 모든 설정을 하나의 gradle파일에 두면, 프로젝트가 커졋을때 관리하기 힘들어집니다.

아래 사진과 같이, 역할별로 gradle파일을 만들고 (메인)build.gradle에 apply를 해주면 관리하기 편해지니 시도해보셔도 좋을거 같습니다.
참고용 레포지토리도 링크로 남겨드립니다

<build.gradle>
apply from: "gradle/db.gradle"
apply from: "gradle/jwt.gradle"
apply from: "gradle/core.gradle"
apply from: "gradle/test.gradle"
apply from: "gradle/slack.gradle"
apply from: "gradle/spring.gradle"
apply from: "gradle/monitor.gradle"
apply from: "gradle/jetbrains.gradle"

오 적용해보겠습니다! 감사합니다!

좋은 팁 감사합니다!!

devxb · 2024-11-06T13:21:07Z

build.gradle

+	implementation 'org.springframework.boot:spring-boot-starter-data-redis'
+
+	// WebFlux 외부 API 호출
+	implementation 'org.springframework.boot:spring-boot-starter-webflux'


mvc와 webflux를 함께 쓰시나요?

이러면 서버는 Tomcat으로 뜨게될까요 Netty로 뜨게될까요?

mvc와 webflux는 어떤상황에 같이 사용할 수 있을까요?

현재 저희 서비스는 WebFlux를 활성화하지 않았기에, Tomcat으로 서버가 구동됩니다.
WebFlux를 추가한 이유는 외부 API(OpenAI, ClovaStudio)를 사용하려고 했기에 추가했습니다!

devxb · 2024-11-06T13:24:17Z

src/main/java/spring/backend/activity/application/CreateQuickStartService.java

+@Service
+@RequiredArgsConstructor
+@Log4j2
+@Transactional


저도 DDD를 좋아하는데, 저는 트랜잭션 관리의 용이성 때문에 domain Service는 domain레이어에 구현하는걸 추천드립니다. (자연스럽게 @Transactional도 domain 밖으로 빠져나오지 않겠죠?)
application 레이어에서는 여러 domain service 를 조합해서 사용해야 하거나 I/O 작업이 발생하는데, 트랜잭션 안에는 넣으면 안되는 상황에 사용합니다.

application service : 여러개의 도메인 서비스의 오케스트레이션 및 트랜잭션분리

domain service : 각 도메인 특정한 로직을 담당

devxb · 2024-11-06T13:30:04Z

src/main/java/spring/backend/activity/domain/value/Keyword.java

+import jakarta.persistence.Enumerated;
+import lombok.*;
+
+@Embeddable


클린아키텍처를 사용한것은 아닌가요?
domain layer가 POJO로 되어있고 의존성도 application -> domain <- infra 로 가고있는거 같아서 클린아키텍처 + DDD라고 생각했는데 여기 jakarta쪽 어노테이션이 있군요

클린아키텍처를 사용하려고 하였으나, 잘 모르는 상황이었어서 사용하지 않았습니다..!
그대신 표현 -> 응용 -> 도메인 <- 인프라 4계층으로 DDD를 설계하려고 하였습니다!

도메인 계층은 POJO로 구현하고 인프라에서 JPA를 사용하였지만,
밸류 타입의 경우 도메인 계층에서 jakarta 어노테이션을 사용하여 POJO로 생성하지 못했습니다..

밸류 타입도 도메인은 POJO로 변경하고, 인프라에 jakarta 어노테이션 사용 및 mapper를 통해 관리하도록 변경하겠습니다!
감사합니다~

devxb · 2024-11-06T14:24:42Z

src/main/java/spring/backend/member/application/ReadMemberHomeService.java

+import spring.backend.activity.query.dao.ActivityDao;
+import spring.backend.activity.query.dao.QuickStartDao;


member -> activity로 의존성이 가고있네요 ㅎㅎ
DDD를 사용해서 도메인별로 로직을 잘 분리해주어도. 사용하는쪽의 의존성을 관리하지 않으면 관리하기 힘든 코드가 될 수 있습니다! (지금 그렇다는건 아니에요)

다른 도메인들끼리는 어떤 방식으로 호출을하는게 좋을지(네트워크 호출? 코드 호출?), 레이어는 어디끼리 참조하고 있어야 좋을지 고민해보셔도 좋을거 같아요.
ex. member.application -> activity.domain?
ex. member.application -> activity.application?

devxb · 2024-11-06T14:26:31Z

src/test/java/spring/backend/activity/dto/request/QuickStartRequestTest.java

+            QuickStartRequest request = new QuickStartRequest(null, 12, 30, "오전", 300, Type.OFFLINE);
+            Set<ConstraintViolation<QuickStartRequest>> violations = validator.validate(request);
+
+            assertThat(violations).isNotEmpty();
+            assertThat(violations).anyMatch(violation -> violation.getMessage().contains("이름은 필수 입력 항목입니다."));


given, when, then을 주석으로라도 명시해놓는게 추후 코드를 다시 읽거나 다른사람이 테스트 코드를 파악할때 도움이 됩니다!

쓰다 안쓰다 했는데 확실하게 써주는게 좋겠네요!
감사합니다 ㅎㅎ

devxb · 2024-11-06T14:28:37Z

src/main/java/spring/backend/recommendation/application/GetRecommendationsFromClovaService.java

+    private static final Pattern CONTENT_PREFIX_PATTERN = Pattern.compile(".*content :");
+    private static final String LINE_SEPARATOR = "\n";
+    private final RecommendationProvider recommendationProvider;
+    public List<ClovaRecommendationResponse> getRecommendationsFromClova(ClovaRecommendationRequest clovaRecommendationRequest) {


요기 함수위에 개행이 빠져있는것 같아요 ㅎㅎ

devxb · 2024-11-06T14:29:57Z

src/main/java/spring/backend/recommendation/infrastructure/clova/application/ClovaService.java

+@Service
+@RequiredArgsConstructor
+@Log4j2
+public class ClovaService {


Service라는 네이밍은 통상적으로 어떤 역할을할때 사용되고 있을지 고민해봐도 좋을거 같습니다.

Service 네이밍은 비즈니스 로직의 중심이 되는 계층이나 애플리케이션의 핵심 기능을 구현하고 관리하는 역할이라 알고있습니다.

코드를 작성하던 당시 추천 목록을 받아오니 핵심 기능을 구현한다 생각해 ClovaService라 지었습니다.

현재 저희 애플리케이션의 메인 로직은 추천 목록을 받아오는 것이 아닌, 받아온 추천 목록을 파싱해 사용자에게 전달하는 것이기에 Service가 아닌 RecommendationProvider로 이름을 변경했습니다.

더 깊게 생각할거리를 주셔서 감사합니다.

devxb · 2024-11-06T14:31:42Z

src/main/java/spring/backend/recommendation/infrastructure/clova/application/ClovaService.java

+            return clovaStudioWebClient.post()
+                    .uri(apiUrl)
+                    .bodyValue(request)
+                    .retrieve()
+                    .bodyToMono(ClovaResponse.class)
+                    .block();
+        } catch (WebClientException e) {


여기서 block을 걸면 reactive의 이점을 누릴 수 있을까요?

reactive가 항상 빠른것은 아니며, 잘못사용하면 오히려 느려지는 경우가 많은데요. 가장 많이 하는 실수중 하나가, reactive 코드에서 blocking I/O를 유발하는 작업을 할때 인 것 같아요.

reactive가 어떻게 성능적인 이점을 주는지, 알아보셔도 좋을거 같아요.

…천활동만 반환한다.

…EN_COOKIE로 변경한다.

devxb reviewed Nov 6, 2024

View reviewed changes

HoyeongJeon and others added 29 commits November 10, 2024 01:02

feat: (#64) 추천 재시도 이후 받은 값 중 일부만 올바른 키워드를 가지고 있는 경우, 올바른 키워드를 가지고있는 추…

6f6591e

…천활동만 반환한다.

feat: (#64) userActivityController의 반환값 이름을 수정한다.

76b41ac

refactor: (#64) 불필요한 조건을 제거한다.

fff4ed6

feat: (#65) QuickStartActivitySelectRequest를 구현한다.

c6edda4

feat: (#65) QuickStartActivitySelectController와 스웨거를 구현한다.

9916d77

feat: (#65) QuickStartActivitySelectService를 생성한다.

b0e8188

feat: (#65) QuickStartActivitySelectSwagger를 만들고 Controller를 구현한다.

b894022

feat: (#65) QuickStartActivitySelectRequest를 구현한다.

1a82e6e

feat: (#65) QuickStartActivitySelectService를 구현한다.

18774bf

feat: (#65) QuickStartActivitySelectServiceTest를 작성한다.

2b96707

feat: (#65) 빠른시작활동선택의 response dto를 만든다.

d933785

feat: (#65) QuickStartActivitySelect는 QuickStartActivityResponse를 반환한다.

d1574c2

fix: (#64) QuickStartActivitySelectSwagger의 중복 PathVariable을 제거한다.

0207071

feat: (#84) Youtube API를 연동한다

e791e45

feat: (#80) MapService 인터페이스를 생성한다.

e603172

feat: (#80) NaverMapErrorCode를 작성한다.

a762af9

feat: (#80) NaverMapService를 구현한다.

0c69114

feat: (#80) NaverMapResponse dto를 생성한다.

a86202b

feat: (#80) NaverMapErrorCode에 파싱 에러를 추가한다.

0a91f7c

feat: (#80) NaverMapResponse로 반환값을 변경한다.

6876c30

refactor: (#80) MapService 를 PlaceInfoProvider로 변경한다.

869af9d

chore: (#88) email 전송을 위한 의존성을 추가한다.

20d5f4a

chore: (#88) MailConfiguration을 작성한다.

5b627c2

feat: (#88) Email 전송을 위한 Util을 구현한다.

e218449

feat: (#88) MailErrorCode를 작성한다.

854b10e

refactor: (#88) EmailUtil의 폴더 구조를 수정하고 예외처리와 로깅을 추가한다.

f2813f3

refactor: (#88) Email 입력 값의 유효성 검사를 추가한다.

9fa71b6

feat: (#88) EmailUtilTest 코드를 작성한다.

72dd285

refactor: (#88) SendEmailRequest 인자 이름을 변경한다.

6132c6f

HoyeongJeon added 28 commits February 12, 2025 17:05

fix: 배포 스크립트의 pull_requests 시 배포되는 부분을 제거한다.

d326445

refactor: AuthenticationErrorCode의 NOT_EXIST_TOKEN의 이름을 NOT_EXIST_TOK…

0417d3f

…EN_COOKIE로 변경한다.

refactor: 불필요한 주석을 제거한다.

f415b63

refactor: 불필요한 반환 값을 제거한다.

c49ff65

refactor: enum 상수 비교를 equals()에서 == 수정한다.(NPE 발생 이슈)

d43eb07

refactor: 코드 컨벤션을 지킨다.

9ca5f1c

refactor: 로그인 리턴값에 대한 새로운 Response Record를 생성한다.

3bedc0e

chore: geoip2 의존성을 생성한다.

7e71850

chore: GeoIpConfiguration을 설정한다.

21e2cbf

feat: 두 좌표 사이 거리 계산을 위한 유틸을 생성한다.

9f9ffbf

feat: 로그인 한 유저의 IP 획득용 ArgumentResolver를 생성한다.

d07d379

feat: argumentResolver를 추가한다.

0125671

feat: GeoLocationService를 구현한다.

50bb600

chore: gitignore를 수정한다.

f826232

feat: refreshToken 저장 시 ip를 함께 저장한다.

0bbab74

feat: 로그인 시 ip를 함께 받아온다.

97a0aa5

feat: 토큰 재발급 시 RTR을 적용한다.

7f084a5

feat: 토큰 재발급 시 100 km 이상에서 들어온 요청에 대해 재발급을 반려한다.

53cb10b

feat: Redis 토큰 저장 메서드를 변경한다.

d4932af

feat: 100km 이상 떨어진 곳에서의 요청 거절 에러를 만든다.

aecca66

feat: 토큰 반환 dto 내용을 변경한다.

28260cc

feat: RotateTokenSwagger 및 Redis 키-밸류 전체 삭제 메서드를 구현한다.

bca23de

feat: 테스트를 작성한다.

6eb35dc

chore: maxmind db를 .gitignore에 추가한다.

08afb01

chore: ci/cd 스크립트를 수정한다.

d13dd1d

refactor: 에러 메시지를 수정한다.

c9bd777

feat: 토큰 재발급 유효 거리를 환경변수로 관리한다.

249c9ee

feat: 쿠키 발급 시 secure, sameSite를 추가한다.

a3b8585

anxi01 deleted the branch review June 30, 2025 18:43

anxi01 closed this Jun 30, 2025

		import spring.backend.activity.query.dao.ActivityDao;
		import spring.backend.activity.query.dao.QuickStartDao;

[C팀] 백엔드파트 코드리뷰용 PR #76

[C팀] 백엔드파트 코드리뷰용 PR #76

Uh oh!

Conversation

anxi01 commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devxb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HoyeongJeon Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anxi01 commented Nov 3, 2024 •

edited

Loading

devxb left a comment •

edited

Loading

HoyeongJeon Nov 26, 2024 •

edited

Loading